fix: store historical rotating quorums by cycle boundary hash by xdustinface · Pull Request #409 · dashpay/rust-dashcore

xdustinface · 2026-02-03T04:12:47Z

This fixes issues in InstantSend lock verification where we miss the selected quorums. It adds infrastructure for storing historical rotating quorums keyed by cycle boundary hash instead of quorum hash. This enables InstantLock verification for previous cycles by:

Adding WORK_DIFF_DEPTH constant (8 blocks offset to cycle boundary)
Adding store_rotating_quorums_for_cycle() to extract and store quorums
Collecting work block hashes from QRInfo diffs (h-4c, h-3c, h-2c, h-c)
Using cycle boundary hash as the key for rotated_quorums_per_cycle

Adds infrastructure for storing historical rotating quorums keyed by cycle boundary hash instead of quorum hash. This enables InstantLock verification for previous cycles by: - Adding `WORK_DIFF_DEPTH` constant (8 blocks offset to cycle boundary) - Adding `store_rotating_quorums_for_cycle()` to extract and store quorums - Collecting work block hashes from `QRInfo` diffs (`h-4c`, `h-3c`, `h-2c`, `h-c`) - Using cycle boundary hash as the key for `rotated_quorums_per_cycle`

coderabbitai · 2026-02-03T04:13:00Z

Warning

Rate limit exceeded

@PastaPastaPasta has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 19 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/store-historical-cycles

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

PastaPastaPasta · 2026-02-03T13:57:18Z

@coderabbitai review

Ensure you take into consideration this DIP https://github.com/dashpay/dips/blob/master/dip-0022.md and it's prior work, to understand system design.

coderabbitai · 2026-02-03T14:00:42Z

🧩 Analysis chain

@PastaPastaPasta, I'll review the PR with DIP-0022 in mind. Let me first fetch the DIP specification and examine the changes.

🏁 Script executed:

#!/bin/bash

# Fetch DIP-0022 content
echo "=== Fetching DIP-0022 ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0022.md

echo -e "\n\n=== Fetching DIP-0006 (Long-Living Masternode Quorums) ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0006.md

echo -e "\n\n=== Checking for DIP-0024 (mentioned in learnings) ==="
curl -s https://raw.githubusercontent.com/dashpay/dips/master/dip-0024.md

Length of output: 84079

🏁 Script executed:

#!/bin/bash

# Get the list of changed files in the PR
echo "=== Changed files in PR ==="
gh pr view 409 --json files --jq '.files[].path'

echo -e "\n\n=== PR diff ==="
gh pr diff 409

Length of output: 8821

🏁 Script executed:

#!/bin/bash

# Find relevant files related to rotating quorums and InstantSend
echo "=== Finding rotating quorum related files ==="
fd -e rs | rg -i "quorum|instant"

Length of output: 812

@PastaPastaPasta, I'll review this PR with DIP-0022 and DIP-0024 specifications in mind. Let me examine the implementation details.

🏁 Script executed:

#!/bin/bash

# Examine the rotated_quorums_per_cycle usage and structure
echo "=== Examining rotated_quorums_per_cycle structure ==="
rg -n -A5 -B5 "rotated_quorums_per_cycle" dash/src/sml/masternode_list_engine/mod.rs | head -100

echo -e "\n\n=== Check InstantLock verification code ==="
cat dash-spv/src/validation/instantlock.rs

Length of output: 12201

🏁 Script executed:

#!/bin/bash

# Look at QRInfo structure to understand the diff components
echo "=== Finding QRInfo structure definition ==="
rg -n "struct.*QRInfo" --type rust -A20

echo -e "\n\n=== Check cycle boundary and work block usage ==="
rg -n "cycle.*boundary|WORK_DIFF_DEPTH" --type rust -C3

Length of output: 10580

🏁 Script executed:

#!/bin/bash

# Examine how qualified quorum entries are used
echo "=== Check QualifiedQuorumEntry structure ==="
rg -n "struct.*QualifiedQuorumEntry" --type rust -A10

echo -e "\n\n=== Check masternode list quorums structure ==="
rg -n "pub quorums:" --type rust -C5

Length of output: 3144

🏁 Script executed:

#!/bin/bash

# Check how verify_is_lock uses the stored quorums
echo "=== Find verify_is_lock implementation ==="
rg -n "fn verify_is_lock" --type rust -A30

echo -e "\n\n=== Check InstantLock structure and cyclehash field ==="
rg -n "struct InstantLock|pub cyclehash" --type rust -C3

Length of output: 5282

🏁 Script executed:

#!/bin/bash

# Check how quorums are retrieved by cycle hash for verification
echo "=== Check quorum retrieval by cycle hash ==="
rg -n "rotated_quorums_per_cycle.*get\(|get.*cyclehash|cycle.*hash.*get" --type rust -C5

echo -e "\n\n=== Check if there are any tests for this functionality ==="
rg -n "#\[test\]|#\[cfg\(test\)\]" dash/src/sml/masternode_list_engine/mod.rs -A3 | head -50

Length of output: 4839

🏁 Script executed:

#!/bin/bash

# Look for any documentation or comments about the cycle boundary vs work block distinction
echo "=== Check comments about cycle boundary and work block ==="
rg -n "cycle.boundary|work.block|h-[0-9]c" --type rust -C2 dash/src/sml/masternode_list_engine/mod.rs

echo -e "\n\n=== Check the old implementation (what was it keyed by before?) ==="
git diff HEAD~1 dash/src/sml/masternode_list_engine/mod.rs | grep -A5 -B5 "rotated_quorums_per_cycle"

Length of output: 8707

@PastaPastaPasta, I've completed my review of PR #409 with DIP-0022 and DIP-0024 in mind. This is an excellent fix that properly implements cycle-based quorum storage for historical InstantLock verification.

✅ Correct Implementation of DIP-0022 & DIP-0024

The PR correctly addresses the requirement that InstantLock messages must be verified using cycleHash rather than quorumHash. Here's what I found:

Key Strengths

WORK_DIFF_DEPTH Constant - Correctly implements the 8-block offset between cycle boundary and work block as specified in DIP-0024
Cycle Boundary Hash as Key - The critical change from using quorum_entry.quorum_hash to cycle_boundary_hash aligns perfectly with DIP-0022's requirement that "any node can follow the steps required to determine the appropriate quorumHash" using the cycleHash
Historical Quorum Storage - Storing quorums from h-c, h-2c, h-3c, and h-4c enables verification of InstantLocks from previous cycles, which is essential for Platform State Transition validation as described in DIP-0022
Dual Storage Strategy:
- Historical cycles: Extracted from masternode lists at work block heights
- Current cycle: Populated from lastCommitmentPerIndex with validation status
This distinction is important because lastCommitmentPerIndex contains "the most recent commitment for each quorumIndex" which may include older quorums if DKG sessions failed

Architecture Observations

Cycle Boundary Calculation

let cycle_boundary_height = work_block_height + WORK_DIFF_DEPTH;

This correctly maps the work block (where masternode lists are selected) to the cycle boundary (where the cycle begins), matching DIP-0024's requirement that "the masternode list at this height is used 8 blocks prior to the current height"

Duplicate Prevention

contains_key() check prevents re-storing historical cycles
clear() call allows current cycle updates on repeated feed_qr_info() calls
This prevents accumulation while allowing state updates

Minor Suggestions

Enhanced Documentation: Consider adding a comment explaining why historical and current quorums use different storage paths (masternode list extraction vs lastCommitmentPerIndex validation)
Test Coverage: Verify tests exist for:
- InstantLock verification using historical cycle hashes
- Correct cycle boundary calculation from work blocks
- Quorum retrieval by cycle hash for different DKG failure scenarios
Feature Flag Clarity: The removal of #[cfg(feature = "quorum_validation")] from the NetworkLLMQExt import is correct since isd_llmq_type() is now used in the non-feature-gated store_rotating_quorums_for_cycle() method

Questions

Edge Case: What happens if feed_qr_info() is called multiple times for the same height h? The clear() prevents accumulation for the current cycle, but are there any performance implications?
Masternode List Availability: The code gracefully handles missing masternode lists with early returns. Are there scenarios where a work block's masternode list might legitimately be absent after applying its diff?

Summary

This PR correctly implements the critical infrastructure needed for deterministic InstantSend lock verification per DIP-0022 and DIP-0024. The shift from quorum hash to cycle hash as the storage key is essential because "any quorum could sign any islock even if it were not responsible to do so for that quorum cycle" under the old approach. The new implementation ensures that "nodes can follow the steps required to determine the appropriate quorumHash and verify the signature" for InstantLocks from any cycle.

🧠 Learnings used

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Maintain backward compatibility for serialized wallets and monitor for new DIPs affecting wallet structure when updating derivation paths

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-16T14:54:24.111Z
Learning: This codebase is not intended for consensus-critical validation; do not rely on exact Dash Core consensus behavior

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: key-wallet/CLAUDE.md:0-0
Timestamp: 2025-12-19T00:07:22.904Z
Learning: Prepare support for future Dash upgrades including Schnorr/Taproot support, descriptor wallets, multi-signature account types, and Lightning Network payment channels

Learnt from: CR
Repo: dashpay/rust-dashcore PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-16T14:53:56.708Z
Learning: This library should NOT be used for consensus code and cannot replicate exact consensus behavior of Dash Core

Learnt from: QuantumExplorer
Repo: dashpay/rust-dashcore PR: 56
File: dash/src/sml/masternode_list_engine/message_request_verification.rs:98-99
Timestamp: 2025-02-27T05:44:42.338Z
Learning: In the Dash codebase, quorum selection for InstantLock verification uses a bit-shifting operation with (64 - n - 1) to extract n bits starting from the second-highest bit of a 64-bit selection hash. The exact reasoning for the extra "-1" isn't documented, but it matches the original C++ implementation for DIP-24 quorum selection.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

xdustinface marked this pull request as ready for review February 3, 2026 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: store historical rotating quorums by cycle boundary hash#409

fix: store historical rotating quorums by cycle boundary hash#409
xdustinface wants to merge 1 commit intov0.42-devfrom
fix/store-historical-cycles

xdustinface commented Feb 3, 2026

Uh oh!

coderabbitai bot commented Feb 3, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

PastaPastaPasta commented Feb 3, 2026

Uh oh!

coderabbitai bot commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

xdustinface commented Feb 3, 2026

Uh oh!

coderabbitai bot commented Feb 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Uh oh!

PastaPastaPasta commented Feb 3, 2026

Uh oh!

coderabbitai bot commented Feb 3, 2026

✅ Correct Implementation of DIP-0022 & DIP-0024

Key Strengths

Architecture Observations

Minor Suggestions

Questions

Summary

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai bot commented Feb 3, 2026 •

edited

Loading